-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: enable bulk getting and unsetting of configuration options #12
Conversation
Additional changes: - Fixes bug with `unset`. '!' does not need to be appended to the end of key names if using `snap unset ...` instead of `snap set ...`. Signed-off-by: Jason C. Nucciarone <[email protected]>
Signed-off-by: Jason C. Nucciarone <[email protected]>
Signed-off-by: Jason C. Nucciarone <[email protected]>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with all the changes, it makes it really easy to get/unset all the slurm config at once.
However, we were planning on adding some default config to fix some slurm limitations for the snap (charmed-hpc/slurm-snap#22), and I worry that using unset
to clear everything could become a pitfall.
Hmm... you make a point. Currently, slurmctld = SlurmManagerBase(ServiceType.SLURMCTLD)
slurmctld.config.unset()
slurmctld.set_defaults() From the snap context, if the entire configuration is blown out we could just add some logic to the |
Yep, sounds good! |
Signed-off-by: Jason C. Nucciarone <[email protected]>
This pull request adds the ability to both bulk get and unset configuration options on slurm config objects. This functionality will be helpful for both if we want to reset the state of Slurm, and quickly get configuration information for various Slurm objects for comparison against new information received by the charms.
For example:
Misc.
.unset(...)
. If we usesnap unset ...
instead ofsnap set ...
, we don't need to append the!
exclamation point to the end of the key name. Whoopsies 🥶ConfigurationManager
now takes a string in its constructor rather than aServiceType
. This makes it much easier to create configuration managers for slurmctld components such ascgroup
andacct_gather
. I realized thatConfigurationManager
only needsconfig_name
, so there's no need to actually pass the fullServiceType
enum.